feat(parser,css_parser): implement checkpoint rewinding #1417
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for biomejs canceled.
|
github-actions
bot
added
A-Parser
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
denbezrukov
reviewed
Jan 3, 2024
| p: &mut CssParser, | ||
| func: impl FnOnce(&mut CssParser) -> Result<T, E>, | ||
| ) -> Result<T, E> { | ||
| let checkpoint = p.checkpoint(); |
There was a problem hiding this comment.
Do we need to implement speculative_parsing?
It blocks a parse recovery since we're going to rewind anyway.
There was a problem hiding this comment.
I suppose it's probably best that we do just for safety. I don't think we need it strictly for how it'll be used here, but definitely no harm in including it. I'll add it in now.
CodSpeed Performance ReportMerging #1417 will not alter performanceFalling back to comparing Summary
|
faultyserver
force-pushed
the
faulty/css-parser-checkpoint
branch
from
273627f to
433c4dd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
From #268 (comment), this implements checkpoint rewinding for the CSS parser to set up for attempting to parse property values and then falling back to generics if the parse fails for any reason.
I think there's another possibility of using
rewrite_eventsthe way that theJsParserdoes to avoid the need for rewinding, but I think it's actually okay to just rewind since there's little cost to it, and we aren't enforcing a particular structure on the re-parsed content (it could be eitherGenericComponentValueListorBogus, we don't actually know at the point of rewinding).Test Plan
Added a test around
try_parseto ensure it rewinds as expected on error and preserves the position on success. Future PRs will start actually using it in the parser logic itself.